Skip to content

fix: Respect --override-* flags on cast call with --trace flag #10721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix: Respect --override-* flags on cast call with --trace flag #10721

wants to merge 4 commits into from

Conversation

Yen
Copy link

@Yen Yen commented Jun 5, 2025

Fixes #10720

grandizzy
grandizzy reviewed Jun 6, 2025
View reviewed changes
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! I think it makes sense, we'd need some tests for it too.
what do others think re adding such?

@Yen
Copy link
Author

Yen commented Jun 6, 2025

I have looked at the existing tests for --override and they use the stderr_eq check to ensure the output is expected. If I add additional tests in the case of --trace this output is more complex and I assume it will change more often. Is it ok for the tests to fail when the format of the trace changes in the future, would the expectation be that the tests are updated to reflect this?

@Yen
Copy link
Author

Yen commented Jun 6, 2025

Ah, actually I did not see the SNAPSHOTS=overwrite functionality which I assume is meant to assist in this case. I will add equivalent tests to the existing --override tests with the --trace flag enabled.

@Yen Yen requested a review from grandizzy June 6, 2025 14:19
@Yen Yen force-pushed the fix/cast-call-trace-overrides branch 2 times, most recently from 007b3e5 to 66a282b Compare June 13, 2025 13:03
@Yen
Copy link
Author

Yen commented Jun 13, 2025

I have added tests and rebased onto the latest master. Thanks

@grandizzy
Copy link
Collaborator

@Yen cast call accepts now block.time and block.number overrides (as of #10727) mind to add those to work with --trace as well? thank you

@Yen Yen force-pushed the fix/cast-call-trace-overrides branch from 66a282b to 5cde566 Compare June 19, 2025 14:58
@Yen
Copy link
Author

Yen commented Jun 19, 2025

I have rebased and added the block overrides to work with --trace. The changes were minor. 👍

@Yen
Copy link
Author

Yen commented Jun 19, 2025

Tests in CI are failing after the rebase but not sure what the reason is as these tests don't use --trace. Can a maintainer confirm these are caused by the changes here as I can't understand what change causes them.

@grandizzy
Copy link
Collaborator

yeah, not related, looks like a contract we use in a test was upgraded and returns different now

grandizzy
grandizzy approved these changes Jun 20, 2025
View reviewed changes
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, makes sense. waiting for one more review before merging, @zerosnacks mind to have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grandizzy grandizzy grandizzy approved these changes

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@yash-atreya yash-atreya Awaiting requested review from yash-atreya yash-atreya is a code owner

@zerosnacks zerosnacks Awaiting requested review from zerosnacks zerosnacks is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Foundry
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug(cast call): --override-* flags not respected with --trace
2 participants
@Yen @grandizzy